-
Notifications
You must be signed in to change notification settings - Fork 99
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Fullname Feature to User Profiles #2575
Add Fullname Feature to User Profiles #2575
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for working on this!
There are some flake8 and linter failures found by the tests
Also you added some files which we don't need in the repository
remove
- .coverage
- dist/mss-9.2.0-py3.11.egg
- instance/mscolab.db
- output_file.py
- pytest.log
There are multiple times added ui_add_user.py, ui_mscolab_connect_dialog.py, ui_mscolab_profile_dialog.py
I have to do a closer look on the other changes,
mslib/mscolab/server.py
Outdated
user_record.nickname = nickname # Update nickname | ||
|
||
# Commit changes to the database | ||
db.session.commit() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all these db commands needs to be moved to the file_manager
likly you can use modify_user
https://github.com/Open-MSS/MSS/blob/develop/mslib/mscolab/file_manager.py#L222
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I need to move all the additional commands to file_manager, and I should revert this part to how it was before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not mandatory. check if you can use file_manager.modify_user you can maybe reuse the existing method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okk
mslib/mscolab/server.py
Outdated
|
||
except Exception as e: | ||
# Log the error message (use logging instead of print for production) | ||
print(f"Error updating user info: {str(e)}") # Replace with proper logging if needed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when you need output, you have to use logging.debug, a print can create a traceback on a webserver
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okk
mslib/msui/mscolab.py
Outdated
@@ -966,6 +966,34 @@ def delete_account(self, _=None): | |||
if r.status_code == 200 and json.loads(r.text)["success"] is True: | |||
self.logout() | |||
|
|||
def editfull_name(self): | |||
if verify_user_token(self.mscolab_server_url, self.token): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a newer syntax for this by a decorator. Have a look on the other lines where that is used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okkkk
mslib/msui/qt5/ui_add_user_dialog.py
Outdated
self.emailid.setPlaceholderText(_translate("addUserDialog", "[email protected]")) | ||
self.passwordLabel.setText(_translate("addUserDialog", "Password:")) | ||
self.password.setPlaceholderText(_translate("addUserDialog", "Your password")) | ||
self.confirmPasswordLabel.setText(_translate("addUserDialog", "Confirm Password:")) | ||
self.rePassword.setPlaceholderText(_translate("addUserDialog", "Confirm your password")) | ||
self.emailIDLabel.setText(_translate("addUserDialog", "Email:")) | ||
self.usernameLabel.setText(_translate("addUserDialog", "Username:")) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this section is not needed, likly this comes from an option of the pyuic5 command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you mean all the changes in ui_add_user_dialog.py is not needed??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add only the fullname
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
|
||
|
||
from . import resources_rc | ||
if __name__ == "__main__": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is also not needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okkk
ui_add_user.py
Outdated
@@ -0,0 +1,10 @@ | |||
# -*- coding: utf-8 -*- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file on wrong place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i removed it.
ui_mscolab_connect_dialog.py
Outdated
@@ -0,0 +1,10 @@ | |||
# -*- coding: utf-8 -*- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file on wrong place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i removed it
ui_mscolab_profile_dialog.py
Outdated
@@ -0,0 +1,10 @@ | |||
# -*- coding: utf-8 -*- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file on wrong place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this also.
.coverage
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need this in the repository
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okk i have removed it..
dist/mss-9.2.0-py3.11.egg
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need this in the repository
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okk i have removed it..
instance/mscolab.db
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need this in the repository
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okk i have removed it..
I won't be able to fully review this until next week, but just a quick comment: we already have a username field, and I understand usernames and nicknames as being the same thing. So I don't see a reason to have two nickname fields when we could just make the username changeable (if it isn't already). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comments
self.deleteAccountBtn.setText(_translate("ProfileWindow", "Delete Account")) | ||
self.uploadImageBtn.setText(_translate("ProfileWindow", "Change Avatar")) | ||
import resources_rc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where does this comes from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
likly you need to convert the ui file with
pyuic5 --from-imports ui_mscolab_profile_dialog.ui > ../qt5/ui_mscolab_profile_dialog.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean? i have converted .ui file to .py file .
mslib/mscolab/server.py
Outdated
user_record.nickname = nickname # Update nickname | ||
|
||
# Commit changes to the database | ||
db.session.commit() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not mandatory. check if you can use file_manager.modify_user you can maybe reuse the existing method
mslib/msui/qt5/ui_add_user_dialog.py
Outdated
self.emailid.setPlaceholderText(_translate("addUserDialog", "[email protected]")) | ||
self.passwordLabel.setText(_translate("addUserDialog", "Password:")) | ||
self.password.setPlaceholderText(_translate("addUserDialog", "Your password")) | ||
self.confirmPasswordLabel.setText(_translate("addUserDialog", "Confirm Password:")) | ||
self.rePassword.setPlaceholderText(_translate("addUserDialog", "Confirm your password")) | ||
self.emailIDLabel.setText(_translate("addUserDialog", "Email:")) | ||
self.usernameLabel.setText(_translate("addUserDialog", "Username:")) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add only the fullname
@@ -247,3 +263,13 @@ def retranslateUi(self, MSColabConnectDialog): | |||
self.idpAuthTokenSubmitBtn.setText(_translate("MSColabConnectDialog", "Submit")) | |||
self.idpAuthTopicLabel.setText(_translate("MSColabConnectDialog", "Identity Provider Authentication")) | |||
self.statusLabel.setText(_translate("MSColabConnectDialog", "Status:")) | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the __name__
section gets in by some conversion option, we don't need that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okk
mslib/index.py
Outdated
@@ -96,10 +95,6 @@ def mss_theme(filename): | |||
|
|||
APP.jinja_env.globals.update(get_topmenu=get_topmenu) | |||
|
|||
@APP.route("/index") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have you moved this? or why is it deleted?
http://localhost:8083/ seems broken.
mslib/mscolab/migrations/versions/a63201ddec7a_to_version_next_major_version.py
Outdated
Show resolved
Hide resolved
output_file.py
Outdated
@@ -0,0 +1,10 @@ | |||
# -*- coding: utf-8 -*- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this?
mslib/msui/mscolab.py
Outdated
self.ui, | ||
self.ui.tr("Edit Full Name"), | ||
self.ui.tr( | ||
f"You're about to change the full name - '{self.active_operation_name}' " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason to add here self.active_operation_name ? How is the users full name linked to an operation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am confused, I need to collect the user's full name and nickname from the UI, send the data to the server, and update the UI with the response from the server. Is this the correct approach?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Start with only adding the full name. See discussion by Matthias.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My question is why is the users fullname related to the operation name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry.. it's not directly related.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Start with only adding the full name. See discussion by Matthias.
where can i find it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mslib/msui/mscolab.py
Outdated
|
||
|
||
def editnick_name(self): | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is currently a stub, or?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not needed, or?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comments/questions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the migration script seems not correct
Are you sure you want to reset the database? This would delete all your data! (y/[n]):y
INFO [alembic.runtime.migration] Context impl SQLiteImpl.
INFO [alembic.runtime.migration] Will assume non-transactional DDL.
INFO [alembic.runtime.migration] Running downgrade 922e4d9c94e2 -> c171019fe3ee, To version 10.0.0
Traceback (most recent call last):
...
KeyError: 'fullname'
you get on this by
annapurna-gupta/MSS$ python mslib/mscolab/mscolab.py db --reset
please also undo your changes to index.py. This is our homepage.
you can see the crash by
annapurna-gupta/MSS$ python mslib/mswms/mswms.py
on your browser http://127.0.0.1:8081
seems I am not allowed to propose changes to your PR, so that is as patch here
|
so should i change migration script by using that command? and I have undone the changes to index.py. |
but the checkbox labeled 'Allow edits from maintainers' is checked. |
mslib/mscolab/migrations/versions/92eaba86a92e_to_version_8_3_5_initial_migration.py
Outdated
Show resolved
Hide resolved
mslib/mscolab/server.py
Outdated
user_record.fullname = fullname # Update full name | ||
|
||
# Commit changes to the database | ||
_handle_db_upgrade().session.commit() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to use the file_manager
self.mscolabURLLabel.setText(_translate("ProfileWindow", "Mscolab")) | ||
self.deleteAccountBtn.setText(_translate("ProfileWindow", "Delete Account")) | ||
self.uploadImageBtn.setText(_translate("ProfileWindow", "Change Avatar")) | ||
|
||
|
||
from . import resources_rc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that is an option of pyuic5 --from-imports
I am still not able to push to your changes/repository: please apply the patch by the patch command,
|
I tried applying the patch, but I can't find the patch file. Could you please share the file or its location so I can proceed? |
It is the blue word above your Posting. Just tested |
i found it, thank you. |
@@ -616,6 +617,15 @@ def set_version_name(): | |||
return jsonify({"success": True, "message": "Successfully set version name"}) | |||
|
|||
|
|||
@APP.route("/edit_user_info", methods=["POST"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is this used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
##Fixes #1282